From 24dcc22614b013e1d6ae5161e259e145195f3aeb Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Sat, 7 Jun 2014 18:29:18 -0300 Subject: [PATCH] Workaround image magick issue with greyscale xcf files IM doesn't seem to properly interpret greyscale xcf files as being greyscale. Tell it to just take the red channel in such a case. Bug: 66323 Change-Id: I46302d43e1029d815be99f481f3942481becd74f --- includes/media/Bitmap.php | 13 ++++ includes/media/XCF.php | 89 ++++++++++++++++++++---- tests/phpunit/includes/media/XCFTest.php | 74 ++++++++++++++++++++ 3 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 tests/phpunit/includes/media/XCFTest.php diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index 44be178cce..76dab03d76 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -355,6 +355,19 @@ class BitmapHandler extends ImageHandler { } } elseif ( $params['mimeType'] == 'image/x-xcf' ) { $animation_post = array( '-layers', 'merge' ); + wfSuppressWarnings(); + $xcfMeta = unserialize( $image->getMetadata() ); + wfRestoreWarnings(); + if ( $xcfMeta + && isset( $xcfMeta['colorType'] ) + && $xcfMeta['colorType'] === 'greyscale-alpha' + && version_compare( $this->getMagickVersion(), "6.8.9-3" ) < 0 + ) { + // bug 66323 - Greyscale images not rendered properly. + // So only take the "red" channel. + $channelOnly = array( '-channel', 'R', '-separate' ); + $animation_post = array_merge( $animation_post, $channelOnly ); + } } // Use one thread only, to avoid deadlock bugs on OOM diff --git a/includes/media/XCF.php b/includes/media/XCF.php index 41e6f036c0..658112ff7c 100644 --- a/includes/media/XCF.php +++ b/includes/media/XCF.php @@ -61,7 +61,28 @@ class XCFHandler extends BitmapHandler { * @return array */ function getImageSize( $image, $filename ) { - return self::getXCFMetaData( $filename ); + $header = self::getXCFMetaData( $filename ); + if ( !$header ) { + return false; + } + + # Forge a return array containing metadata information just like getimagesize() + # See PHP documentation at: http://www.php.net/getimagesize + $metadata = array(); + $metadata[0] = $header['width']; + $metadata[1] = $header['height']; + $metadata[2] = null; # IMAGETYPE constant, none exist for XCF. + $metadata[3] = sprintf( + 'height="%s" width="%s"', $header['height'], $header['width'] + ); + $metadata['mime'] = 'image/x-xcf'; + $metadata['channels'] = null; + $metadata['bits'] = 8; # Always 8-bits per color + + assert( '7 == count($metadata); ' . + '# return array must contains 7 elements just like getimagesize() return' ); + + return $metadata; } /** @@ -124,23 +145,61 @@ class XCFHandler extends BitmapHandler { wfDebug( __METHOD__ . ": canvas size of '$filename' is {$header['width']} x {$header['height']} px\n" ); - # Forge a return array containing metadata information just like getimagesize() - # See PHP documentation at: http://www.php.net/getimagesize + return $header; + } + + /** + * Store the channel type + * + * Greyscale files need different command line options. + * + * @param File $file The image object, or false if there isn't one. + * Warning, FSFile::getPropsFromPath might pass an (object)array() instead (!) + * @param string $path The filename + * @return string + */ + public function getMetadata( $file, $filename ) { + $header = self::getXCFMetadata( $filename ); $metadata = array(); - $metadata[0] = $header['width']; - $metadata[1] = $header['height']; - $metadata[2] = null; # IMAGETYPE constant, none exist for XCF. - $metadata[3] = sprintf( - 'height="%s" width="%s"', $header['height'], $header['width'] - ); - $metadata['mime'] = 'image/x-xcf'; - $metadata['channels'] = null; - $metadata['bits'] = 8; # Always 8-bits per color + if ( $header ) { + // Try to be consistent with the names used by PNG files. + // Unclear from base media type if it has an alpha layer, + // so just assume that it does since it "potentially" could. + switch( $header['base_type'] ) { + case 0: + $metadata['colorType'] = 'truecolour-alpha'; + break; + case 1: + $metadata['colorType'] = 'greyscale-alpha'; + break; + case 2: + $metadata['colorType'] = 'index-coloured'; + break; + default: + $metadata['colorType'] = 'unknown'; - assert( '7 == count($metadata); ' . - '# return array must contains 7 elements just like getimagesize() return' ); + } + } else { + // Marker to prevent repeated attempted extraction + $metadata['error'] = true; + } + return serialize( $metadata ); + } - return $metadata; + /** + * Should we refresh the metadata + * + * @param File $file The file object for the file in question + * @param string $metadata Serialized metadata + * @return bool One of the self::METADATA_(BAD|GOOD|COMPATIBLE) constants + */ + public function isMetadataValid( $file, $metadata ) { + if ( !$metadata ) { + // Old metadata when we just put an empty string in there + return self::METADATA_BAD; + } else { + return self::METADATA_GOOD; + } } /** diff --git a/tests/phpunit/includes/media/XCFTest.php b/tests/phpunit/includes/media/XCFTest.php new file mode 100644 index 0000000000..ae4fa8bc22 --- /dev/null +++ b/tests/phpunit/includes/media/XCFTest.php @@ -0,0 +1,74 @@ +handler = new XCFHandler(); + } + + + /** + * @param string $filename + * @param int $expectedWidth width + * @param int $expectedHeigh height + * @dataProvider provideGetImageSize + * @covers XCFHandler::getImageSize + */ + public function testGetImageSize( $filename, $expectedWidth, $expectedHeight ) { + $file = $this->dataFile( $filename, 'image/x-xcf' ); + $actual = $this->handler->getImageSize( $file, $file->getLocalRefPath() ); + $this->assertEquals( $expectedWidth, $actual[0] ); + $this->assertEquals( $expectedHeight, $actual[1] ); + } + + public static function provideGetImageSize() { + return array( + array( '80x60-2layers.xcf', 80, 60 ), + array( '80x60-RGB.xcf', 80, 60 ), + array( '80x60-Greyscale.xcf', 80, 60 ), + ); + } + + /** + * @param string $metadata Serialized metadata + * @param int $expected One of the class constants of XCFHandler + * @dataProvider provideIsMetadataValid + * @covers XCFHandler::isMetadataValid + */ + public function testIsMetadataValid( $metadata, $expected ) { + $actual = $this->handler->isMetadataValid( null, $metadata ); + $this->assertEquals( $expected, $actual ); + } + + public static function provideIsMetadataValid() { + return array( + array( '', XCFHandler::METADATA_BAD ), + array( serialize( array( 'error' => true ) ), XCFHandler::METADATA_GOOD ), + array( false, XCFHandler::METADATA_BAD ), + array( serialize( array( 'colorType' => 'greyscale-alpha' ) ), XCFHandler::METADATA_GOOD ), + ); + } + + /** + * @param string $filename + * @param string $expected Serialized array + * @dataProvider provideGetMetadata + * @covers XCFHandler::getMetadata + */ + public function testGetMetadata( $filename, $expected ) { + $file = $this->dataFile( $filename, 'image/png' ); + $actual = $this->handler->getMetadata( $file, "$this->filePath/$filename" ); + $this->assertEquals( $expected, $actual ); + } + + public static function provideGetMetadata() { + return array( + array( '80x60-2layers.xcf', 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ), + array( '80x60-RGB.xcf', 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ), + array( '80x60-Greyscale.xcf', 'a:1:{s:9:"colorType";s:15:"greyscale-alpha";}' ), + ); + } +} -- 2.20.1